Therese API Happy thought #9
Conversation
…e and patch. Updated json with id
…ith all the endpoints
jszor
left a comment
jszor
left a comment
There was a problem hiding this comment.
Hey Therese! Good job with this project :) One suggestion to improve the readability + maintainability of the code could be to make it a little more modular by extracting some of the code into separate files/folders like I suggested with the async await functions in your routes, or even the schemas/models can probably be in a "models" folder as separate files as well. Other than that great job with the routes and error handling! 🙌
data/dogs.json
Outdated
There was a problem hiding this comment.
You can probably safely delete this file from your Github, since it's not actually related to the happy thoughts project. Just so you don't have superfluous code in your repo :)
package.json
Outdated
| "@babel/core": "^7.17.9", | ||
| "@babel/node": "^7.16.8", | ||
| "@babel/preset-env": "^7.16.11", | ||
| "bcrypt-nodejs": "^0.0.3", |
There was a problem hiding this comment.
Just out of personal curiosity, do you know what this particular package is for? I just noticed I don't have it in my packages but everything still works (I think? 😅). I only have the one called bcrypt...
server.js
Outdated
| const Thought = mongoose.model("Thought", thoughtSchema) | ||
| const User = mongoose.model("User", userSchema) | ||
|
|
||
| if (process.env.RESET_THOUGHTS) { |
There was a problem hiding this comment.
I see that your function here is supposed to seed your database using "thoughtsData" that you haven't actually imported and for which there is no associated file in your repo. I'm guessing it's because you want to start with a blank database maybe? If that's the case then this function can probably just be removed since it doesn't serve a purpose 😊
| }) | ||
| }) | ||
|
|
||
| app.get("/thoughts", async (req, res) => { |
There was a problem hiding this comment.
This is just a tip that I got from Simon and that I found quite useful for decluttering the code and making it visually more readable, which personally helps me a lot, but there is obviously no right or wrong here. I just thought I would share the tip in case you hadn't considered it:
Instead of having all your async await functions in your server.js file, you could consider extracting them into separate files and just importing them from those files into your server.js. So instead your app.get route to fetch all your thoughts could look like this:
app.get("/thoughts", getThoughts)
And then you could have your getThoughts.js file in a folder with all of your other functions.
| type: String, | ||
| required: true, | ||
| unique: true, | ||
| match: /.+\@.+\..+/, |
There was a problem hiding this comment.
Cool that you challenged yourself to use regex here :)
package.json
Outdated
| "dotenv": "^16.5.0", | ||
| "express": "^4.17.3", | ||
| "express-list-endpoints": "^7.1.1", | ||
| "jsonwebtoken": "^9.0.2", |
There was a problem hiding this comment.
Looks like you have jsonwebtoken as part of your dependencies but you haven't actually used jsonwebtoken in the code. Maybe at that point it's best to remove it from the dependencies so that the node modules for jsonwebtoken aren't downloaded if they are not needed?
server.js
Outdated
|
|
||
| }) | ||
| } catch (error) { | ||
| if (existingUser) { |
There was a problem hiding this comment.
I'm a little bit confused here. You seem to be checking to see if existingUser is true, but you have never defined existingUser. Maybe I'm missing something?
JennieDalgren
left a comment
JennieDalgren
left a comment
There was a problem hiding this comment.
Great start with this project.
There are a few things left to do.
I see that you have implemented all endpoints needed, however deleting and updating can be done without being logged in. This should not be doable.
There are a few things in your code, that also Juan pointed out, that can be removed because it is unused. The dogs data and the seedfunction with a non existing thoughts variable.
Also i cant see all this new features reflected in your frontend? sign up and login and delete and edit thoughts can't be done in you frontend but only from postman.
Take a look at the requirements in Disco again and fix these things before I can give you an approved (G) on the project!
Keep up the good work 💪
… and update thought. Fixed the check for existingUser.
3 participants
Please include your Render link here.
https://lillebrorgrodas-first-api.onrender.com/
All functionality hasn't been implemented yet.